Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 170-178: The diagnostic handler should validate that params
contains string fields "level" and "message" before emitting; update the code in
adapterclient.cpp where params and QJsonObject obj are used to check
obj.contains("level") && obj.value("level").isString() and similarly for
"message", and if either check fails call qCWarning(scopeComm) with a clear
message (including the raw params) and return without emitting
diagnosticReceived; otherwise call diagnosticReceived with the verified string
values (use toString() only after isString()).

---

Nitpick comments:
In `@tests/communication/tst_modbuspoll.cpp`:
- Around line 9-19: Add a resetCaptured() helper that sets g_capturedType to a
default and clears g_capturedMessage, and call resetCaptured() at the start of
each test that uses captureHandler (e.g., TestModbusPoll::diagnosticDebugLevel
and other TestModbusPoll tests) so stale global values cannot make a test pass;
update tests to invoke resetCaptured() before
qInstallMessageHandler(captureHandler) and before exercising the code under
test.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@adapters/describe.json`:
- Line 15: The defaults in adapters/describe.json for the fields persistent and
int32LittleEndian were changed to true while adapters/json-rpc-spec.md still
documents them as false, causing an API mismatch for adapter.describe; fix by
making them consistent: either revert the two occurrences of persistent and
int32LittleEndian in adapters/describe.json back to false to match the spec, or
update adapters/json-rpc-spec.md to state the new true defaults—ensure you
update both places where those keys appear in adapters/describe.json and update
the adapter.describe section of the spec accordingly so runtime behavior and
documentation match.

In `@src/communication/modbuspoll.cpp`:
- Around line 123-149: The onAdapterDiagnostic function currently misses
handling the protocol "error" level causing those messages to hit the fallback;
add an else-if branch that checks for level == QStringLiteral("error") and route
it to qCCritical(scopeAdapter) << message (use qCCritical to preserve proper
severity), leaving other branches and the fallback unchanged so unknown levels
still warn; update only the ModbusPoll::onAdapterDiagnostic function and
reference the scopeAdapter logging category when adding the mapping.

In `@src/ProtocolAdapter/adapterclient.h`:
- Around line 117-120: Update the docstring for the signal that is emitted on
adapter.diagnostic notifications (the comment block above the signal declaration
in adapterclient.h) to include the missing "error" severity level; change the
severity list from "debug", "info", or "warning" to include "error" (e.g.,
"debug", "info", "warning", or "error") so the documentation matches the current
protocol.

In `@tests/communication/tst_modbuspoll.cpp`:
- Around line 36-82: Before invoking onAdapterDiagnostic in each TestModbusPoll
test (diagnosticDebugLevel, diagnosticInfoLevel, diagnosticWarningLevel,
diagnosticUnknownLevel), reset the shared capture state g_capturedType and
g_capturedMessage to their default/empty values so prior test output cannot mask
failures; do this immediately before qInstallMessageHandler(captureHandler)
(references: g_capturedType, g_capturedMessage, captureHandler, _pModbusPoll,
onAdapterDiagnostic).
- Line 26: The test enables the wrong logging category so adapter debug messages
never reach the handler; update the QLoggingCategory::setFilterRules call used
in the test setup to include the adapter category (e.g., enable
scope.comm.adapter.debug=true or a wildcard like scope.comm.*.debug=true) so the
messages emitted by onAdapterDiagnostic (category scope.comm.adapter) are
delivered and the debug-level assertions become reliable.

---

Duplicate comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 176-178: The code currently reads `params.toObject()` and blindly
calls `toString()` on `obj.value("level")` and `obj.value("message")`; validate
that both keys exist and that `obj.value(QStringLiteral("level")).isString()`
and `obj.value(QStringLiteral("message")).isString()` before converting to
QString and calling `emit diagnosticReceived(... )`; if either check fails,
avoid emitting a potentially misleading empty/invalid diagnostic (log or emit an
explicit malformed-diagnostic warning instead). Ensure you update the logic
around the `QJsonObject obj = params.toObject();` block and the `emit
diagnosticReceived` call so the values are type-checked first and only converted
when valid.

---

Nitpick comments:
In `@tests/communication/tst_modbuspoll.cpp`:
- Around line 72-82: Add a new unit test mirroring
TestModbusPoll::diagnosticUnknownLevel that invokes the same onAdapterDiagnostic
method on _pModbusPoll but passes QStringLiteral("error") as the diagnostic
level and an appropriate message, installs captureHandler via
qInstallMessageHandler, restores the previous handler, and then asserts the
expected behavior (e.g., QCOMPARE on g_capturedType and QVERIFY on
g_capturedMessage) to validate the protocol-defined "error" level path;
reference TestModbusPoll::diagnosticUnknownLevel and the onAdapterDiagnostic
invocation to implement the new test.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 117-119: Do not set _state = State::IDLE immediately after calling
_pProcess->stop(); instead leave the state as STOPPING/ERROR until the child
process actually exits, and only transition to State::IDLE inside
onProcessFinished() after processFinished() is observed; update the three
locations that currently clear _state (the Unexpected non-object path emitting
sessionError, the timeout path that emits sessionStopped(), and the other
error/stop branches referenced) to call _pProcess->stop() without resetting
_state, ensure timers/signals are not prematurely stopped so onProcessFinished()
still receives the completion event, and adjust prepareAdapter()/State::STOPPING
handling so fast retries cannot start a new process while the old process is
still running.

In `@src/ProtocolAdapter/adapterprocess.cpp`:
- Around line 51-59: Replace the QTimer::singleShot lambda with a cancellable
timer and a named handler: add an int member (e.g., m_stopTimerId = 0) and use
startTimer(cStopTimeoutMs) instead of QTimer::singleShot to store the returned
timer id; implement a private handler (e.g., void handleStopTimeout() or
override timerEvent and dispatch when timerId == m_stopTimerId) that checks
_pProcess->state() != QProcess::NotRunning and calls _pProcess->kill(); ensure
you call killTimer(m_stopTimerId) and reset m_stopTimerId to 0 from start() and
onProcessFinished() to cancel any pending stop timeout before restarting or
after exit, and remove the lambda to satisfy the "no multi-statement lambda"
guideline.

In `@tests/ProtocolAdapter/tst_adapterprocess.cpp`:
- Around line 47-50: Replace the fixed QTest::qWait(500) sleeps in
tst_adapterprocess.cpp with signal-driven waits: after calling
AdapterProcess::stop() use a QSignalSpy that watches the process finished signal
(the same signal used in processFinishedEmittedOnStop()) and call
spy.wait(cStopTimeoutMs) so the test waits up to cStopTimeoutMs (3000 ms)
instead of a fixed 500ms; similarly, in sendRequestEmitsResponseReceived()
replace the fixed qWait with a QSignalSpy on the response-received signal used
by the test and call spy.wait(cStopTimeoutMs). Ensure you reference the same
signal names the tests currently expect and remove the QTest::qWait calls.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 165-179: In onHandshakeTimeout(), don't emit sessionStopped
immediately after calling _pProcess->stop() because stop() is non-blocking;
instead set _state = State::STOPPING (or set a new bool like _handshakeTimedOut
= true) before calling _pProcess->stop(), call _pProcess->stop(), and remove the
immediate emit; then update onProcessFinished() to check the new flag/_state and
emit sessionStopped only once the process has actually finished, and if the
handshake timed out but an error condition should be reported emit sessionError
there instead (use symbols onHandshakeTimeout, _pProcess->stop(), _state,
_handshakeTimedOut (or State::STOPPING), onProcessFinished, sessionStopped, and
sessionError).

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 117-120: The code sets _state = State::IDLE before calling
_pProcess->stop() and even emits sessionStopped in the timeout/STOPPING path,
which allows re-entry while the old process is still alive; change the logic so
you do NOT set _state to State::IDLE or emit sessionStopped before the process
exit is observed: call _pProcess->stop() without changing _state (leave it in
STOPPING or a dedicated terminal state), remove any premature sessionStopped
emission in the timeout/STOPPING path, and ensure that the only place that sets
_state = State::IDLE and emits sessionStopped is onProcessFinished() after
confirming the process has exited.
- Around line 198-200: The code currently reads level/message from params
without type checks and silently converts non-strings to empty values before
calling diagnosticReceived; update the handler around QJsonObject obj =
params.toObject() to validate that obj.value(QStringLiteral("level")) and
obj.value(QStringLiteral("message")) both .isString() before calling emit
diagnosticReceived(...), and if either check fails, do not emit
diagnosticReceived and instead log a warning (e.g., qWarning()) that includes
the invalid payload (params) so malformed adapter.diagnostic messages are
dropped and visible for debugging.

---

Nitpick comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 7-8: Add a short Doxygen block immediately above the
AdapterClient::AdapterClient(AdapterProcess* pProcess, QObject* parent, int
handshakeTimeoutMs) definition in the .cpp file: include a one-line `@brief`
describing the constructor purpose and brief `@param` tags for pProcess, parent
and handshakeTimeoutMs (e.g., handshake timeout in milliseconds), and, if
relevant, an `@note` about ownership/lifetime of pProcess; place it directly above
the constructor signature so the source-level documentation guideline for public
functions is satisfied.